core, eth: improve delivery speed on header requests #23105
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This may actually make the performance a bit worse, in the case that a peer is 3-4 blocks behind us, and wants to catch up: requesting a few headers close to our tip (or beyond). Previously, we would have the headers in object-form in a header cache, whereas this PR makes them read from db. I'll look into it |
Pushed a commit to use the headerchain cache better. |
s1na
reviewed
Jun 25, 2021
Kampoz18
approved these changes
Jun 25, 2021
|
Triage discussion: probably safe, but we should hold with this until after the London release. |
karalabe
reviewed
Jul 6, 2021
|
TODO (not in this PR), add basefee check to sanityCheck |
karalabe
reviewed
Aug 3, 2021
holiman
commented
Aug 3, 2021
holiman
force-pushed
the
faster_headers
branch
2 times, most recently
from
7cecb9c
to
5d34b01
Compare
|
Rebased on top of the eth/66 changes that got merged. This still does not make use of the faster ancient accessors though |
holiman
force-pushed
the
faster_headers
branch
from
5d34b01
to
3d65612
Compare
holiman
force-pushed
the
faster_headers
branch
from
2897f56
to
8114ca2
Compare
|
Rebased again. Also, tests are added to check the correctness of the split leveldb/ancient loader |
holiman
force-pushed
the
faster_headers
branch
from
8114ca2
to
e61b5de
Compare
|
This is now rebased on top of #23566, which should go in first |
holiman
commented
Oct 25, 2021
fjl
reviewed
Oct 25, 2021
holiman
force-pushed
the
faster_headers
branch
from
e61b5de
to
c89118f
Compare
|
Rebased + robustified |
holiman
force-pushed
the
faster_headers
branch
from
77b7f9f
to
24948ab
Compare
|
Rebased (Part V) |
|
I tested this a bit on a live node. On my node, I made it
Example: I then collected some stats about the performance. It doesn't matter a whole lot which handler was first, for serving Also, no difference in the two responses have been found so far |
|
After letting it run for a bit longer: |
holiman
force-pushed
the
faster_headers
branch
from
24948ab
to
b80cb55
Compare
ivanshukhov
pushed a commit
to Kapoio/go-kapoio
that referenced
this pull request
Dec 11, 2021
This PR reduces the amount of work we do when answering header queries, e.g. when a peer is syncing from us. For some items, e.g block bodies, when we read the rlp-data from database, we plug it directly into the response package. We didn't do that for headers, but instead read headers-rlp, decode to types.Header, and re-encode to rlp. This PR changes that to keep it in RLP-form as much as possible. When a node is syncing from us, it typically requests 192 contiguous headers. On master it has the following effect: - For headers not in ancient: 2 db lookups. One for translating hash->number (even though the request is by number), and another for reading by hash (this latter one is sometimes cached). - For headers in ancient: 1 file lookup/syscall for translating hash->number (even though the request is by number), and another for reading the header itself. After this, it also performes a hashing of the header, to ensure that the hash is what it expected. In this PR, I instead move the logic for "give me a sequence of blocks" into the lower layers, where the database can determine how and what to read from leveldb and/or ancients. There are basically four types of requests; three of them are improved this way. The fourth, by hash going backwards, is more tricky to optimize. However, since we know that the gap is 0, we can look up by the parentHash, and stlil shave off all the number->hash lookups. The gapped collection can be optimized similarly, as a follow-up, at least in three out of four cases. Co-authored-by: Felix Lange <fjl@twurst.com> (cherry picked from commit db03faa)
JacekGlen
pushed a commit
to JacekGlen/go-ethereum
that referenced
this pull request
May 26, 2022
This PR reduces the amount of work we do when answering header queries, e.g. when a peer is syncing from us. For some items, e.g block bodies, when we read the rlp-data from database, we plug it directly into the response package. We didn't do that for headers, but instead read headers-rlp, decode to types.Header, and re-encode to rlp. This PR changes that to keep it in RLP-form as much as possible. When a node is syncing from us, it typically requests 192 contiguous headers. On master it has the following effect: - For headers not in ancient: 2 db lookups. One for translating hash->number (even though the request is by number), and another for reading by hash (this latter one is sometimes cached). - For headers in ancient: 1 file lookup/syscall for translating hash->number (even though the request is by number), and another for reading the header itself. After this, it also performes a hashing of the header, to ensure that the hash is what it expected. In this PR, I instead move the logic for "give me a sequence of blocks" into the lower layers, where the database can determine how and what to read from leveldb and/or ancients. There are basically four types of requests; three of them are improved this way. The fourth, by hash going backwards, is more tricky to optimize. However, since we know that the gap is 0, we can look up by the parentHash, and stlil shave off all the number->hash lookups. The gapped collection can be optimized similarly, as a follow-up, at least in three out of four cases. Co-authored-by: Felix Lange <fjl@twurst.com>
This was referenced Sep 23, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR reduces the amount of work we do when answering header queries, e.g. when a peer is syncing from us.
192contiguous headers. On master it has the following effect:2db lookups. One for translating hash->number (even though the request is by number), and another for reading by hash (this latter one is sometimes cached).1file lookup/syscall for translating hash->number (even though the request is by number), and another for reading the header itself. After this, it also performes a hashing of the header, to ensure that the hash is what it expected.In this PR, I instead move the logic for "give me a sequence of blocks" into the lower layers, where the database can determine how and what to read from leveldb and/or ancients.
There are basically four types of requests; three of them are improved this way. The fourth,
by hash going backwards, is more tricky to optimize. However, since we know that thegapis0, we can look up by theparentHash, and stlil shave off all the number->hash lookups.The gapped collection can be optimized similarly, as a follow-up, at least in three out of four cases.